Skip to content

Slider: fix duplicated property editor settings properties#22898

Merged
nielslyngsoe merged 4 commits into
release/17.4.1from
v17/hotfix/slider-remove-duplicate-range-minimum-property
May 19, 2026
Merged

Slider: fix duplicated property editor settings properties#22898
nielslyngsoe merged 4 commits into
release/17.4.1from
v17/hotfix/slider-remove-duplicate-range-minimum-property

Conversation

@nielslyngsoe

Copy link
Copy Markdown
Member

Removes duplicate Property Editor settings and adds a client-side error when it happens.

Copilot AI review requested due to automatic review settings May 19, 2026 13:13
@claude

claude Bot commented May 19, 2026

Copy link
Copy Markdown

Claude finished @nielslyngsoe's task in 1m 59s —— View job


PR Review

Target: origin/release/17.4.1 · Based on commit: be30228e

Removes the duplicate minimumRange property setting from the Slider UI manifest (it was already defined in the schema manifest), adds a defensive duplicate-alias guard in the data type workspace context, and improves the minimumRange description.

  • Other changes: Slider "Minimum range" PE setting description is now populated (was empty string); the duplicate check in #mergeConfigProperties() is a general guard that will fire for any property editor with duplicate aliases, not just slider.

Suggestions

  • data-type-workspace.context.ts:263: throw new Error(...) is called inside an RxJS subscriber callback (via observe()). If source.subscribe(callback) propagates an uncaught synchronous throw, it may crash the workspace rather than showing a developer-friendly error. For this type of defensive/configuration guard, console.error() paired with a user-facing notification would be more resilient — the workspace could still render while surfacing the misconfiguration. Since the slider bug itself is fixed, this only matters if another property editor introduces the same mistake in future.

    // More resilient alternative:
    if (aliasSet.has(setting.alias)) {
        console.error(`Duplicate alias "${setting.alias}" in Property Editor configuration.`);
        // optionally: UmbNotificationContext notification
        continue; // or break, depending on preferred behaviour
    }

Approved with Suggestions for improvement

Good to go, but please carefully consider the importance of the suggestions.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes duplicate minimumRange settings property on the Slider property editor (declared both in the schema manifest and the UI manifest) and adds a defensive client-side check in the data type workspace that throws when duplicate setting aliases are merged from schema/UI/data-source manifests.

Changes:

  • Remove duplicated minimumRange property and its default value from the Slider UI manifest, keeping the canonical definition on the schema manifest (with the descriptive text moved to the schema).
  • Add duplicate-alias detection in UmbDataTypeWorkspaceContext.#mergeConfigProperties that throws when two merged settings share an alias.
  • Minor refactor in #transferConfigDefaultData: rename the loop variable from defaultDataItem to property and remove an outdated TODO comment.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Umbraco.Web.UI.Client/src/packages/property-editors/slider/Umbraco.Slider.ts Adds a meaningful description to the schema's minimumRange property.
src/Umbraco.Web.UI.Client/src/packages/property-editors/slider/manifests.ts Removes the duplicate minimumRange property and default-data entry from the UI manifest.
src/Umbraco.Web.UI.Client/src/packages/data-type/workspace/data-type-workspace.context.ts Adds duplicate-alias detection on merged settings and renames a loop variable for clarity.

@claude claude Bot added area/frontend category/ux User experience category/ui User interface labels May 19, 2026
@nielslyngsoe nielslyngsoe changed the title fix duplicate slider pe-settings properties Slider: fix duplicated property editor settings properties May 19, 2026
@nielslyngsoe nielslyngsoe enabled auto-merge (squash) May 19, 2026 13:24
@nielslyngsoe nielslyngsoe merged commit ba29b91 into release/17.4.1 May 19, 2026
26 of 27 checks passed
@nielslyngsoe nielslyngsoe deleted the v17/hotfix/slider-remove-duplicate-range-minimum-property branch May 19, 2026 14:19
AndyButland pushed a commit that referenced this pull request May 19, 2026
* fix duplicate slider pe-settings properties

* remove comment

* avoid throws
alexsee pushed a commit to alexsee/umbraco-container that referenced this pull request Jun 15, 2026
Updated
[Umbraco.Cms.Persistence.Sqlite](https://github.com/umbraco/Umbraco-CMS)
from 17.4.0 to 17.4.2.

<details>
<summary>Release notes</summary>

_Sourced from [Umbraco.Cms.Persistence.Sqlite's
releases](https://github.com/umbraco/Umbraco-CMS/releases)._

## 17.4.2

## What's Changed
### 🐛 Bug Fixes
* Cache: Only write to url table on a single server in load balanced
environments to remove lock contention by @​nikolajlauridsen in
umbraco/Umbraco-CMS#22890
* Cache: Add scope-level cache version tier to reduce DB hits in bulk
operation by @​nikolajlauridsen in
umbraco/Umbraco-CMS#22563
* Migrations: Add auto upgrade coordination for load-balanced setups by
@​nikolajlauridsen in umbraco/Umbraco-CMS#22815

**Full Changelog**:
umbraco/Umbraco-CMS@release-17.4.1...release-17.4.2

## 17.4.1

## What's Changed

### 🐛 Bug Fixes
* Content Workspace: Load Data-Types based on Loaded Content Types by
@​nielslyngsoe in umbraco/Umbraco-CMS#22886
* Output Caching: Correctly gate auto-registration of `UseOutputCache()`
middleware by @​AndyButland in
umbraco/Umbraco-CMS#22897
* Slider: fix duplicated property editor settings properties by
@​nielslyngsoe in umbraco/Umbraco-CMS#22898

**Full Changelog**:
umbraco/Umbraco-CMS@release-17.4.0...release-17.4.1

Commits viewable in [compare
view](umbraco/Umbraco-CMS@release-17.4.0...release-17.4.2).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=Umbraco.Cms.Persistence.Sqlite&package-manager=nuget&previous-version=17.4.0&new-version=17.4.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
alexsee pushed a commit to alexsee/umbraco-container that referenced this pull request Jun 15, 2026
Updated
[Umbraco.Cms.DevelopmentMode.Backoffice](https://github.com/umbraco/Umbraco-CMS)
from 17.4.0 to 17.4.2.

<details>
<summary>Release notes</summary>

_Sourced from [Umbraco.Cms.DevelopmentMode.Backoffice's
releases](https://github.com/umbraco/Umbraco-CMS/releases)._

## 17.4.2

## What's Changed
### 🐛 Bug Fixes
* Cache: Only write to url table on a single server in load balanced
environments to remove lock contention by @​nikolajlauridsen in
umbraco/Umbraco-CMS#22890
* Cache: Add scope-level cache version tier to reduce DB hits in bulk
operation by @​nikolajlauridsen in
umbraco/Umbraco-CMS#22563
* Migrations: Add auto upgrade coordination for load-balanced setups by
@​nikolajlauridsen in umbraco/Umbraco-CMS#22815

**Full Changelog**:
umbraco/Umbraco-CMS@release-17.4.1...release-17.4.2

## 17.4.1

## What's Changed

### 🐛 Bug Fixes
* Content Workspace: Load Data-Types based on Loaded Content Types by
@​nielslyngsoe in umbraco/Umbraco-CMS#22886
* Output Caching: Correctly gate auto-registration of `UseOutputCache()`
middleware by @​AndyButland in
umbraco/Umbraco-CMS#22897
* Slider: fix duplicated property editor settings properties by
@​nielslyngsoe in umbraco/Umbraco-CMS#22898

**Full Changelog**:
umbraco/Umbraco-CMS@release-17.4.0...release-17.4.1

Commits viewable in [compare
view](umbraco/Umbraco-CMS@release-17.4.0...release-17.4.2).
</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants